Skip to content

Conversation

@bokelley
Copy link
Contributor

@bokelley bokelley commented Nov 7, 2025

Summary

Fixes AttributeError in v1.0.3 where response parser couldn't handle MCP SDK's Pydantic TextContent objects, and refactors the architecture to establish proper separation of concerns.

Problem

The MCP SDK returns Pydantic objects (e.g., TextContent), not plain dicts. In v1.0.3, response parsing logic was added that used item.get("type") and item.get("text"), which only works on dicts:

AttributeError: 'TextContent' object has no attribute 'get'

This was a regression - v1.0.2 worked correctly.

Root Cause

The MCP adapter was passing MCP SDK-specific Pydantic objects directly to the generic response parser, violating protocol boundary separation. The parser had to handle both parsing AND protocol translation.

Solution

Architectural fix with proper separation of concerns:

  1. MCP Adapter (mcp.py): Added _serialize_mcp_content() to convert Pydantic objects to plain dicts at the protocol boundary
  2. Response Parser (response_parser.py): Simplified to only handle plain dicts, removed dual-mode helper function
  3. Tests: Added serialization tests to protocol adapter, removed Pydantic mocks from parser tests

Benefits

  • ✅ Clean separation: adapters translate, parsers parse
  • ✅ Simpler type signatures: list[dict[str, Any]] (no more | Any)
  • ✅ Parser tests use plain dicts (no MCP SDK dependency)
  • ✅ More obvious code (removed clever helper function)
  • ✅ Follows adapter pattern correctly
  • ✅ Establishes pattern for future MCP content types (images, resources)

Commits

  1. Initial fix: Handle Pydantic objects with get_field() helper (f29179b)
  2. Architectural refactor: Move serialization to protocol boundary (7d2a521)

Tests

Response Parser Tests:

  • ✅ Parsing dict-based MCP text content
  • ✅ Multiple content items
  • ✅ Empty content error handling
  • ✅ Invalid schema error handling
  • ✅ Empty text content skipping

Protocol Adapter Tests:

  • ✅ Serializing dict content (pass-through)
  • ✅ Serializing Pydantic v2 objects (model_dump)
  • ✅ Serializing mixed content (dicts + Pydantic)

All tests passing (15/15 for affected modules).

Test plan

  • Run unit tests for response_parser (all passing)
  • Run unit tests for protocol adapters (all passing)
  • Verify serialization handles Pydantic v2 model_dump()
  • Verify backward compatibility with dict-based tests
  • Test with real MCP agent to verify fix works in production

🤖 Generated with Claude Code

bokelley and others added 3 commits November 6, 2025 21:06
The MCP SDK returns Pydantic TextContent objects, not plain dicts.
The response_parser.py was using .get() method which only works on dicts,
causing AttributeError: 'TextContent' object has no attribute 'get'.

This change adds a helper function that handles both dict and Pydantic
object access patterns, ensuring compatibility with both the MCP SDK's
actual return types and the test suite's mock dicts.

Tests added for:
- Pydantic TextContent objects
- Mixed dict and Pydantic content
- Empty Pydantic text content handling

Fixes regression introduced in v1.0.3 where parsing logic was added
that assumed dict objects.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
This refactor addresses the architectural issue identified in code review:
the response parser was handling both parsing AND protocol translation,
violating single responsibility principle.

Changes:
1. Added _serialize_mcp_content() to MCPAdapter to convert Pydantic
   objects to dicts at the protocol boundary
2. Simplified parse_mcp_content() to only handle plain dicts
3. Removed get_field() helper function (no longer needed)
4. Removed Pydantic mock tests from response_parser tests
5. Added serialization tests to protocol adapter tests

Benefits:
- Clean separation: adapters translate, parsers parse
- Simpler type signatures: list[dict[str, Any]] instead of | Any
- Tests use plain dicts (no MCP SDK dependency in parser tests)
- More obvious code (removed clever helper function)
- Follows adapter pattern correctly

The fix maintains backward compatibility while establishing proper
architectural boundaries for future MCP content types (images, resources).

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
Mypy now correctly infers types after hasattr and callable checks,
making the type: ignore[attr-defined] comment unnecessary.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@bokelley bokelley merged commit 6b60365 into main Nov 7, 2025
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants